feat(treasury): index governor votes and tallies #175
Conversation
Track LivepeerGovernor votes with per-voter state and proposal-level aggregates. Co-authored-by: kyriediculous <vergauwennico@gmail.com>
|
🚀 Subgraph Studio preview deployed
curl -H 'Content-Type: application/json' \
-d '{"query":"{ protocol(id: \"0\") { inflation } }"}' \
https://api.studio.thegraph.com/query/31909/livepeer-ci/pr-175-c6ca542-20762221469 |
jmulq
left a comment
jmulq
left a comment
There was a problem hiding this comment.
Left a few comments. Nothing major but I'd definitely look at changing stuff around the enum comments. Has this been deployed and is it syncing without errors?
| vote.voter = account.id; | ||
| } | ||
|
|
||
| vote.support = supportLabel; |
There was a problem hiding this comment.
Not sure we should be using the string here, and instead should use the generated enum type (TreasuryVoteSupport.For or something I believe).
This would also mean we can tweak supportFromValue and increaseProposalTotals. Relying on the enum type instead of string values feels like a more appropriate way to handle this.
There was a problem hiding this comment.
@jmulq I looked into this while working on the PR. Per the Graph docs on enums https://thegraph.com/docs/en/subgraphs/guides/enums/, enum fields in mappings are set using their string values, and the generated AssemblyScript entities expose them as string, not typed enums. AssemblyScript/codegen also don't currently support string enums (see AssemblyScript/assemblyscript#560).
I might be missing something, but if not, would d39ea1a be an acceptable workaround for now?
There was a problem hiding this comment.
Hey @rickstaa - apologies for the confusion. I've been developing in subtreams world recently and wrongly said we can use the generated types. These aren't available in the handlers as you correctly mentioned.
There are similar workaround to yours in this example repo - https://github.com/chidubemokeke/Subgraph-Tutorial-Enums/tree/main.
However, I am happy for it to be the original block of code as well. Whatever you guys find easier to reason about.
Ensure the schema includes all required ABIs and entities referenced throughout the treasury mapping.
Remove incorrect entity directive from the TreasuryVoteSupport enum. Co-authored-by: jmulq <james.mulqueeny@outlook.com>
Centralize treasury support strings in a namespace. Graph entities store enums as strings and AS/codegen don’t expose string enums right now. Co-authored-by: jmulq <james.mulqueeny@outlook.com>
|
@jmulq thanks again for the thorough review. I’ve addressed most of the comments. The only one I couldn’t fully resolve yet is the enum-related point. Could you take a look at the proposed solution and let me know if it works for you, or if you see a better approach? Thanks in advance. |
|
@mehrdadmms here is the current state of this pull request. This pull request can be merged and reviewed after @jmulq has responded to #175 (comment) and approved it. After this is done you can work with @yondonfu to deploy it and unblock the explorer team. |
jmulq
left a comment
jmulq
left a comment
There was a problem hiding this comment.
Approving this so long as the Livepeer team are happy with the decided implementation for the enum logic. Feel free to revert it back to the prior code, or use the implemented workaround.
This pull erquest ensures LivepeerGovernor votes are tracked with per-voter state and proposal-level aggregates.